Prevent Caliper events from dying on problem-group pseudo source files#3058
Conversation
Problems selected from a problem group have the pseudo source file "group:problemGroupName" (see WeBWorK::Utils::Instructor::assignProblemToUserSetVersion), which is not a real file under the course templates directory. Caliper::Entity::problem unconditionally passed that path to WeBWorK::Utils::Tags->new, which dies when it cannot open the file, so any Caliper event embedding the problem entity (e.g. answer submission in a gateway quiz that uses problem groups) crashed server side. Skip tag parsing for "group:" pseudo source files instead. For those problems the entity now reports keywords => [] and tags => undef. Real problem files are unaffected: their keywords and unblessed tags are built exactly as before.
drgrice1
left a comment
There was a problem hiding this comment.
I suppose this is okay for now.
You should be forewarned that I have entirely rewritten the Caliper code. There are lots of issues with it. One is that its code is all over in the webwork2 code base, and it is something that is rarely used, and it carries overhead even if not used. My rewrite moves it into a plugin that will not even be used unless the server administrator enables it. The code is generally a kludge as well. It is poorly written and is a mess. (Things like you did even in your changes such as adding unnecessary empty parenthesis after object method calls, and quoting hash keys.) I have completely cleaned it up and fixed other issues with it.
I have integrated this change into my rewrite. I will most likely wait until the next release for my rewrite.
|
We have been using this in our instance for the past year. Plugin sound a great idea. Looking forward to your rewrite! |
Summary
Prevent Caliper events from dying on problem-group pseudo source files.
Problems selected from a problem group carry the pseudo source file
group:problemGroupName(seeWeBWorK::Utils::Instructor::assignProblemToUserSetVersion), which is not a real file under the course templates directory.Caliper::Entity::problemunconditionally passed that path toWeBWorK::Utils::Tags->new, whoseopen ... or diekills the request. As a result, any Caliper event that embeds the problem entity — for example an answer-submission event in a gateway quiz built from problem groups — crashed server side whenever Caliper is enabled.This change guards the tag-parsing block in
sub problemso it is skipped forgroup:pseudo source files. For those problems the entity payload now reportskeywords => []andtags => undef. Real problem files are unaffected:WeBWorK::Utils::Tagsinitializes{keywords}to[]for every real.pgfile it parses, sokeywordsis always an arrayref, and the keyword trimming and unblessed tag hash are built exactly as before.Note that
sub problem_userintentionally keeps the unguarded tag parsing: user problems for versioned (gateway) sets are assigned a concrete source file selected out of the group at set-version creation time, so a merged user problem never carries thegroup:pseudo path.Testing
Caliper::Entity::problemwith the dependency surface stubbed (WeBWorK::DB,Caliper::ResourceIri, and aWeBWorK::Utils::Tagsstub reproducing the realopen ... or diebehavior):group:myGroupdies withcannot open .../group:myGroup: No such file or directory; after this change it returns a payload withkeywords => [],extensions.tags => undef, andextensions.source_filepassed through..pgsource file produces an identical payload before and after the change (keywords trimmed of quotes/whitespace, unblessed tags hash populated).group:) source file still dies as before; this change does not swallow genuine errors.perl -c lib/Caliper/Entity.pmpasses (with dependency stubs, since a full course environment is not available in the check environment).perltidy(Perl::Tidy 20260204, matchingcheck-formats.yml) with the repo.perltidyrcproduces no changes.Dependencies
Depends on #3050 (this branch is based on it, since #3050 already touches the same region of
lib/Caliper/Entity.pmwith theCaliper::ResourceIripackage rename and theshowHintsAfter()fix).